feat: improve layout on smaller screens #1285
Conversation
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughAdds a new responsive layout hook and integrates it across multiple screens and components to compute padding, spacing, and typography based on device dimensions and safe-area insets. Several layouts switch to dynamic values; one screen reworks content into a KeyboardAvoidingView + ScrollView. Minor prop extensions and memoization added. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Screen as Screen Component
participant Hook as useCompactLayout
participant RN as React Native Dimensions
participant SafeArea as Safe Area Insets
Screen->>Hook: useCompactLayout(options)
Hook->>RN: useWindowDimensions()
Hook-->>Screen: { isCompact*, selectResponsiveValue(s), getResponsiveHorizontalPadding() }
Screen->>SafeArea: useSafeAreaInsets()
Screen->>Screen: Compute paddings, gaps, font sizes via selectors
Screen->>UI: Render with responsive styles (padding, gap, fontSize)
note over Screen,UI: Buttons, typography, and containers adapt to width/height
alt Keyboard visible (RecoverWithPhrase)
Screen->>UI: KeyboardAvoidingView + ScrollView layout
else No keyboard
Screen->>UI: Static YStack/BOTTOM sections with responsive padding
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
transphorm
changed the title
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting
| styles.topSection, | ||
| roundTop && styles.roundTop, | ||
| roundTop ? { marginTop: top } : { paddingTop: top }, | ||
| spacingStyle, |
There was a problem hiding this comment.
The new responsive padding in TopSection is applied after the safe-area padding (paddingTop: top). Because spacingStyle sets paddingVertical, it overwrites paddingTop, so screens that render ExpandableBottomLayout.TopSection without roundTop no longer include the safe area inset and their content will be rendered under the status bar on devices with a notch. Consider combining the safe-area value into the responsive padding instead of replacing it.
Useful? React with 👍 / 👎.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
app/src/layouts/ExpandableBottomLayout.tsx (1)
62-71: Consolidate useCompactLayout calls to avoid duplication.Both
TopSectionandBottomSectionindependently calluseCompactLayout({ compactHeight: 760 })and create similarspacingStylecomputations. This creates unnecessary duplication and could lead to inconsistencies if the threshold or spacing logic changes.Consider computing the responsive values once at the parent
ExpandableBottomLayoutlevel and passing them down as props, or extract the spacing logic into a shared hook/constant.Example consolidation:
+const useExpandableBottomSpacing = () => { + const { selectResponsiveValue } = useCompactLayout({ + compactHeight: 760, + }); + return useMemo( + () => ({ + paddingHorizontal: selectResponsiveValue(16, 20, 'width'), + paddingVertical: selectResponsiveValue(16, 20, 'height'), + paddingTop: selectResponsiveValue(18, 30, 'height'), + }), + [selectResponsiveValue], + ); +}; + const TopSection: React.FC<TopSectionProps> = ({ children, backgroundColor, ...props }) => { const { top } = useSafeAreaInsets(); const { roundTop, style: incomingStyle, ...restProps } = props; - const { selectResponsiveValue } = useCompactLayout({ - compactHeight: 760, - }); - const spacingStyle = useMemo( - () => ({ - paddingHorizontal: selectResponsiveValue(16, 20, 'width'), - paddingVertical: selectResponsiveValue(16, 20, 'height'), - }), - [selectResponsiveValue], - ); + const spacing = useExpandableBottomSpacing(); + const spacingStyle = { + paddingHorizontal: spacing.paddingHorizontal, + paddingVertical: spacing.paddingVertical, + };Also applies to: 124-133
app/src/screens/app/LaunchScreen.tsx (1)
44-45: Document hardcoded layout multipliers.Several magic numbers are used without explanation:
- Line 44:
screenWidth * 0.8(80% of screen width) and max width of320- Line 45:
180 / 300aspect ratio (0.6) for card dimensions- Line 63:
screenHeight * 0.08(8% of screen height) for top paddingWhile the responsive approach using
Math.minandMath.maxprovides safety bounds, these multipliers should be extracted as named constants with comments explaining the design rationale. This improves maintainability and makes it easier to adjust the layout if design requirements change.Example:
+// Design system constants +const CARD_WIDTH_PERCENT = 0.8; // 80% of screen width for optimal card presentation +const CARD_MAX_WIDTH = 320; // Cap at 320px to prevent over-sizing on tablets +const CARD_ASPECT_RATIO = 180 / 300; // Original design aspect ratio +const TOP_PADDING_PERCENT = 0.08; // 8% of screen height + const LaunchScreen: React.FC = () => { // ... - const cardWidth = Math.min(screenWidth * 0.8, 320); - const cardHeight = cardWidth * (180 / 300); + const cardWidth = Math.min(screenWidth * CARD_WIDTH_PERCENT, CARD_MAX_WIDTH); + const cardHeight = cardWidth * CARD_ASPECT_RATIO; // ... - const topPadding = Math.max(screenHeight * 0.08, baseTopPadding); + const topPadding = Math.max(screenHeight * TOP_PADDING_PERCENT, baseTopPadding);Also applies to: 63-64
app/src/screens/account/recovery/RecoverWithPhraseScreen.tsx (1)
54-56: Extract magic layout values into named constants and verify layout on all device sizes.
- In RecoverWithPhraseScreen.tsx (line 56) and LaunchScreen.tsx (line 63), replace inline
160,0.3, and0.08with descriptive constants (e.g.MIN_TEXTAREA_HEIGHT,TEXTAREA_HEIGHT_RATIO,TOP_PADDING_RATIO) and document their rationale.- Manually test the KeyboardAvoidingView + ScrollView layout on small, standard, and large devices to confirm the TextArea scales correctly.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (12)
app/src/components/Mnemonic.tsx(5 hunks)app/src/hooks/useCompactLayout.ts(1 hunks)app/src/layouts/ExpandableBottomLayout.tsx(6 hunks)app/src/screens/account/recovery/AccountRecoveryScreen.tsx(3 hunks)app/src/screens/account/recovery/RecoverWithPhraseScreen.tsx(4 hunks)app/src/screens/app/LaunchScreen.tsx(5 hunks)app/src/screens/documents/selection/CountryPickerScreen.tsx(6 hunks)app/src/screens/documents/selection/IDPickerScreen.tsx(6 hunks)app/src/screens/onboarding/AccountVerifiedSuccessScreen.tsx(4 hunks)app/src/screens/onboarding/DisclaimerScreen.tsx(4 hunks)app/src/screens/onboarding/SaveRecoveryPhraseScreen.tsx(4 hunks)app/src/screens/shared/ComingSoonScreen.tsx(8 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{js,ts,tsx,jsx,sol,nr}
📄 CodeRabbit inference engine (.cursorrules)
**/*.{js,ts,tsx,jsx,sol,nr}: NEVER log sensitive data including PII (names, DOB, passport numbers, addresses), credentials, tokens, API keys, private keys, or session identifiers.
ALWAYS redact/mask sensitive fields in logs using consistent patterns (e.g.,***-***-1234for passport numbers,J*** D***for names).
Files:
app/src/layouts/ExpandableBottomLayout.tsxapp/src/screens/onboarding/AccountVerifiedSuccessScreen.tsxapp/src/screens/account/recovery/AccountRecoveryScreen.tsxapp/src/components/Mnemonic.tsxapp/src/screens/onboarding/DisclaimerScreen.tsxapp/src/screens/onboarding/SaveRecoveryPhraseScreen.tsxapp/src/screens/documents/selection/CountryPickerScreen.tsxapp/src/screens/account/recovery/RecoverWithPhraseScreen.tsxapp/src/screens/app/LaunchScreen.tsxapp/src/screens/documents/selection/IDPickerScreen.tsxapp/src/hooks/useCompactLayout.tsapp/src/screens/shared/ComingSoonScreen.tsx
app/**/*.{ts,tsx}
📄 CodeRabbit inference engine (app/AGENTS.md)
Type checking must pass before PRs (yarn types)
Files:
app/src/layouts/ExpandableBottomLayout.tsxapp/src/screens/onboarding/AccountVerifiedSuccessScreen.tsxapp/src/screens/account/recovery/AccountRecoveryScreen.tsxapp/src/components/Mnemonic.tsxapp/src/screens/onboarding/DisclaimerScreen.tsxapp/src/screens/onboarding/SaveRecoveryPhraseScreen.tsxapp/src/screens/documents/selection/CountryPickerScreen.tsxapp/src/screens/account/recovery/RecoverWithPhraseScreen.tsxapp/src/screens/app/LaunchScreen.tsxapp/src/screens/documents/selection/IDPickerScreen.tsxapp/src/hooks/useCompactLayout.tsapp/src/screens/shared/ComingSoonScreen.tsx
app/src/**/*.{ts,tsx,js,jsx}
⚙️ CodeRabbit configuration file
app/src/**/*.{ts,tsx,js,jsx}: Review React Native TypeScript code for:
- Component architecture and reusability
- State management patterns
- Performance optimizations
- TypeScript type safety
- React hooks usage and dependencies
- Navigation patterns
Files:
app/src/layouts/ExpandableBottomLayout.tsxapp/src/screens/onboarding/AccountVerifiedSuccessScreen.tsxapp/src/screens/account/recovery/AccountRecoveryScreen.tsxapp/src/components/Mnemonic.tsxapp/src/screens/onboarding/DisclaimerScreen.tsxapp/src/screens/onboarding/SaveRecoveryPhraseScreen.tsxapp/src/screens/documents/selection/CountryPickerScreen.tsxapp/src/screens/account/recovery/RecoverWithPhraseScreen.tsxapp/src/screens/app/LaunchScreen.tsxapp/src/screens/documents/selection/IDPickerScreen.tsxapp/src/hooks/useCompactLayout.tsapp/src/screens/shared/ComingSoonScreen.tsx
🧬 Code graph analysis (10)
app/src/screens/onboarding/AccountVerifiedSuccessScreen.tsx (3)
app/src/mocks/react-native-safe-area-context.js (1)
useSafeAreaInsets(44-46)packages/mobile-sdk-alpha/src/constants/colors.ts (1)
white(59-59)packages/mobile-sdk-alpha/src/components/typography/Title.tsx (1)
Title(18-41)
app/src/screens/account/recovery/AccountRecoveryScreen.tsx (2)
app/src/mocks/react-native-safe-area-context.js (1)
useSafeAreaInsets(44-46)packages/mobile-sdk-alpha/src/components/typography/Title.tsx (1)
Title(18-41)
app/src/components/Mnemonic.tsx (1)
packages/mobile-sdk-alpha/src/constants/colors.ts (3)
slate300(37-37)white(59-59)slate500(43-43)
app/src/screens/onboarding/DisclaimerScreen.tsx (1)
app/src/screens/verification/ProofRequestStatusScreen.tsx (1)
styles(326-342)
app/src/screens/onboarding/SaveRecoveryPhraseScreen.tsx (1)
packages/mobile-sdk-alpha/src/components/typography/Title.tsx (1)
Title(18-41)
app/src/screens/documents/selection/CountryPickerScreen.tsx (4)
packages/mobile-sdk-alpha/src/components/layout/YStack.tsx (1)
YStack(10-12)packages/mobile-sdk-alpha/src/components/typography/BodyText.tsx (1)
BodyText(12-14)packages/mobile-sdk-alpha/src/utils/fonts.ts (2)
advercase(7-7)dinot(8-8)packages/mobile-sdk-alpha/src/constants/colors.ts (2)
slate500(43-43)black(8-8)
app/src/screens/account/recovery/RecoverWithPhraseScreen.tsx (2)
app/src/mocks/react-native-safe-area-context.js (1)
useSafeAreaInsets(44-46)packages/mobile-sdk-demo/tests/mocks/react-native.ts (1)
Platform(14-25)
app/src/screens/app/LaunchScreen.tsx (1)
app/src/mocks/react-native-gesture-handler.ts (1)
GestureDetector(45-50)
app/src/screens/documents/selection/IDPickerScreen.tsx (5)
app/src/components/NavBar/DocumentFlowNavBar.tsx (1)
DocumentFlowNavBar(14-46)packages/mobile-sdk-alpha/src/components/layout/YStack.tsx (1)
YStack(10-12)packages/mobile-sdk-alpha/src/utils/fonts.ts (1)
dinot(8-8)packages/mobile-sdk-alpha/src/constants/colors.ts (1)
black(8-8)packages/mobile-sdk-alpha/src/components/typography/BodyText.tsx (1)
BodyText(12-14)
app/src/screens/shared/ComingSoonScreen.tsx (1)
app/src/mocks/react-native-safe-area-context.js (1)
useSafeAreaInsets(44-46)
🪛 GitHub Actions: Workspace CI
app/src/components/Mnemonic.tsx
[error] 5-5: simple-import-sort/imports: Run autofix to sort these imports!
🪛 GitHub Check: workspace-lint
app/src/layouts/ExpandableBottomLayout.tsx
[failure] 5-5:
Run autofix to sort these imports!
app/src/screens/account/recovery/AccountRecoveryScreen.tsx
[warning] 30-30:
Insert ⏎···
[warning] 83-83:
Replace ·gap={buttonStackGap}·width="100%"·paddingTop={buttonPaddingTop} with ⏎············gap={buttonStackGap}⏎············width="100%"⏎············paddingTop={buttonPaddingTop}⏎··········
app/src/components/Mnemonic.tsx
[failure] 5-5:
Run autofix to sort these imports!
[warning] 115-115:
Replace ⏎············key={i}⏎············word={word}⏎············index={i}⏎············compact={isCompactWidth}⏎········· with ·key={i}·word={word}·index={i}·compact={isCompactWidth}
app/src/screens/onboarding/SaveRecoveryPhraseScreen.tsx
[failure] 5-5:
Run autofix to sort these imports!
app/src/screens/account/recovery/RecoverWithPhraseScreen.tsx
[warning] 132-132:
Replace ⏎··········styles.layout,⏎··········{·paddingBottom:·bottom·+·24·},⏎········ with styles.layout,·{·paddingBottom:·bottom·+·24·}
app/src/hooks/useCompactLayout.ts
[warning] 86-86:
Replace infer·TValue with ⏎··········infer·TValue⏎········
[warning] 84-84:
Delete ]
[warning] 82-82:
Replace keyof·TConfig,⏎········ResponsiveValueConfig<unknown>, with [keyof·TConfig,·ResponsiveValueConfig<unknown>]
[warning] 81-81:
Delete [
[warning] 50-50:
Replace options?:·ResponsivePaddingOptions with ⏎····options?:·ResponsivePaddingOptions,⏎··
[warning] 43-43:
Replace TConfig·extends·Record<string,·ResponsiveValueConfig<unknown>> with ⏎····TConfig·extends·Record<string,·ResponsiveValueConfig<unknown>>,⏎··
[failure] 9-9:
Expected DEFAULT_COMPACT_HEIGHT before DEFAULT_COMPACT_WIDTH
⏰ Context from checks skipped due to timeout of 300000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: android-build-test
- GitHub Check: e2e-ios
- GitHub Check: analyze-ios
- GitHub Check: analyze-android
- GitHub Check: build-deps
🔇 Additional comments (1)
app/src/screens/account/recovery/AccountRecoveryScreen.tsx (1)
42-51: Omittingdimensionis valid for selectResponsiveValues. The hook defaultsdimensionto'any', applying the compact layout when either width or height is compact. No changes needed.Likely an incorrect or invalid review comment.
| {index} | ||
| </Text> | ||
| <Text color={slate500} fontSize={14} fontWeight={500}> | ||
| <Text color={slate500} fontSize={compact ? 13 : 14} fontWeight={500}> | ||
| {word} | ||
| </Text> |
There was a problem hiding this comment.
Fix mnemonic numbering regression
Line 44 renders {index}, but the list now passes index={i} (Lines 115-121), so the first pill shows 0 instead of 1. That’s a user-visible correctness bug. Display index + 1 (or pass i + 1) before rendering.
- <Text color={slate300} fontSize={compact ? 13 : 14} fontWeight={500}>
- {index}
+ <Text color={slate300} fontSize={compact ? 13 : 14} fontWeight={500}>
+ {index + 1}Also applies to: 115-121
🤖 Prompt for AI Agents
In app/src/components/Mnemonic.tsx around lines 44 to 48 (and similarly where
the list passes index at lines 115-121), the component renders the zero-based
`index` which displays "0" for the first mnemonic; change the displayed value to
`index + 1` (or alternatively pass `i + 1` from the parent) so numbering starts
at 1, and update the list invocation at lines 115-121 to pass a 1-based index if
you choose that approach.
Summary
useCompactLayoutto compute multiple responsive values at onceTesting
https://chatgpt.com/codex/tasks/task_b_68eebf896864832d9480ccc839ea9595
Summary by CodeRabbit
New Features
Refactor
Style